Skip to content

Detect MOVBE #1356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 5, 2023
Merged

Detect MOVBE #1356

merged 2 commits into from
Jan 5, 2023

Conversation

calebzulawski
Copy link
Member

No description provided.

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@thomcc
Copy link
Member

thomcc commented Nov 17, 2022

Since this is insta-stable, does this need an FCP?

Also, I'm not sure that the since is right.

@calebzulawski
Copy link
Member Author

calebzulawski commented Nov 17, 2022

I'm not really sure about how an FCP or the attribute works in this case. MOVBE itself was already stabilized, detection was omitted for whatever reason.

@thomcc
Copy link
Member

thomcc commented Nov 17, 2022

It might just be an oversight and not need FCP then. I suspect the since should still be the actual version, though. I think it can be the reviewer's call either way, but just felt it's worth mentioning.

@Amanieu
Copy link
Member

Amanieu commented Nov 18, 2022

I'd change the since to the current rustc version (1.67) since that is more accurate, even though movbe_target_feature was indeed stabilized in 1.34.

Otherwise LGTM. I'll FCP just in case but this should be pretty uncontroversial.

@rfcbot fcp merge

@Amanieu
Copy link
Member

Amanieu commented Nov 18, 2022

Unless of course, rfcbot doesn't work in this repo, in which case I'll just ping @rust-lang/libs-api to see if anyone has any objections.

@rfcbot
Copy link

rfcbot commented Nov 18, 2022

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@dtolnay
Copy link
Member

dtolnay commented Dec 20, 2022

I'm marking @yaahc's box because she has stepped down from T-libs-api after the point that this feature got proposed for FCP.

@rfcbot
Copy link

rfcbot commented Dec 20, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

psst @Amanieu, I wasn't able to add the final-comment-period label, please do so.

@dtolnay dtolnay added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Dec 20, 2022
@rfcbot
Copy link

rfcbot commented Dec 30, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

psst @Amanieu, I wasn't able to add the finished-final-comment-period label, please do so.

@Amanieu
Copy link
Member

Amanieu commented Jan 5, 2023

Note that movbe_target_feature is still unstable in rustc, it should probably be stabilized there as well.

@Amanieu Amanieu merged commit 6d0d7ef into rust-lang:master Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants